Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough여러 컴포저블의 UI/상태 관리와 공개 API를 변경합니다. SearchBarTopSheet의 파라미터명 변경 및 새 콜백 추가, BottomSheet 관련 상태·스크림·레이아웃 조정, 일부 컴포저블 가시성 축소와 레이아웃/아이콘/상호작용 개선이 포함됩니다. (요약 50단어 이내) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/file/src/main/java/com/example/file/ui/bottom/sheet/TopFolderEditBottomSheet.kt (1)
25-31: Non-null assertion (!!) 사용으로 인한 잠재적 NPE 위험
folderStateViewModel.readyToUpdateTopFolder!!.folderName에서!!연산자를 사용하고 있습니다.onColorIdDeliver콜백이 실행되는 시점에readyToUpdateTopFolder가 null일 경우 앱이 크래시됩니다.안전한 처리를 위해 null 체크를 추가하거나 early return 패턴을 사용하는 것을 권장합니다.
🔧 안전한 null 처리 제안
onColorIdDeliver = { colorId -> + val topFolder = folderStateViewModel.readyToUpdateTopFolder ?: return@TextFieldFileBottomSheet fileViewModel.updateCategoryColor( - categoryName = folderStateViewModel.readyToUpdateTopFolder!!.folderName, + categoryName = topFolder.folderName, colorId = (colorId + 1).toLong(), colorStyle = CategoryColorStyle.categoryStyleList[colorId] ) },feature/file/src/main/java/com/example/file/ui/bottom/sheet/NewBottomFolderBottomSheet.kt (1)
13-20:NewBottomFolderBottomSheet에서sheetState파라미터 누락 - 다른 폴더 관련 바텀 시트와 일관성 부족
BottomFolderEditBottomSheet,TopFolderEditBottomSheet,ShareBottomSheet,LinkCategorizationBottomSheet는 모두sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true)를 명시적으로 전달하여 바텀 시트가 부분 확장 상태를 건너뛰고 바로 전체 확장되도록 합니다.
NewBottomFolderBottomSheet는sheetState를 전달하지 않아TextFieldFileBottomSheet의 기본값(skipPartiallyExpanded = false)을 사용하므로, 이 컴포넌트만 부분 확장 상태를 허용합니다.동일한 목적의 다른 폴더 관련 바텀 시트들과 동작을 맞추기 위해
sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true)를 전달하는 것을 권장합니다.
🤖 Fix all issues with AI agents
In `@design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt`:
- Around line 149-156: The debounce coroutine restarts because
LaunchedEffect(text) relaunches on every text change; change it to
LaunchedEffect(Unit) so the coroutine starts once and uses snapshotFlow to
observe text changes, keeping the existing snapshotFlow { text }. Ensure you
keep the chain .map { it.trim() }.filter { it.length >= 2
}.debounce(350).distinctUntilChanged().collectLatest(onQueryChange) and remove
the text key from LaunchedEffect so debounce works as intended with
onQueryChange.
- Around line 123-128: The LaunchedEffect(visible) in SearchBarTopSheet calls
resetAndDismiss() when visible becomes false, which can cause onDismiss() to be
invoked twice; change the behavior so the effect only resets internal state and
does not call onDismiss(): either remove or refactor the onDismiss() call out of
resetAndDismiss() (e.g., create a dedicated resetState() used by the
LaunchedEffect) and keep onDismiss() invocation only at the external dismiss
entrypoint; update calls to use resetState()/resetAndDismiss() accordingly and
retain names resetAndDismiss() and onDismiss() for clarity.
In `@feature/file/build.gradle.kts`:
- Line 56: The project mixes Compose BOM and an explicit version for
foundation.layout; decide one approach and make it consistent: either remove the
explicit version reference for foundation.layout (remove
version.ref="foundationLayout" on
implementation(libs.androidx.compose.foundation.layout) so the
androidx.compose.bom manages it) or remove the androidx.compose.bom dependency
and keep the explicit foundation.layout version; update the implementation entry
for foundation.layout or the BOM declaration accordingly and ensure
imports/usages (e.g., Box, Column, Row, Arrangement, padding) continue to
compile.
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.kt`:
- Around line 197-201: The clickable menu item using noRippleClickable (where
enabled = selectedOption != selectedText and onClick = onClick) lacks an
explicit accessibility role; update the call to pass the semantics role (e.g.,
Role.Button) — i.e., add the role parameter to noRippleClickable so TalkBack
users receive the proper element role (you can also include onClickLabel if
desired for a more descriptive announcement).
🧹 Nitpick comments (9)
gradle/libs.versions.toml (1)
34-34: foundation과 foundation-layout 버전 불일치.
foundation버전(1.9.5, Line 33)과foundationLayout버전(1.10.0)이 다릅니다. 두 라이브러리는 같은 Compose Foundation 모듈 계열이므로 버전이 일치해야 호환성 문제를 방지할 수 있습니다.Compose BOM(
composeBom = "2024.09.00")을 사용 중이라면, BOM이 버전을 자동으로 관리하도록 개별 버전 지정 없이 사용하는 것을 권장합니다.♻️ BOM 사용 시 권장 수정안
BOM을 통해 버전을 관리하려면:
-androidx-compose-foundation-layout = { group = "androidx.compose.foundation", name = "foundation-layout", version.ref = "foundationLayout" } +androidx-compose-foundation-layout = { group = "androidx.compose.foundation", name = "foundation-layout" }또는 명시적 버전을 유지하려면 foundation과 동일한 버전으로 통일:
-foundationLayout = "1.10.0" +foundationLayout = "1.9.5"design/src/main/java/com/example/design/modifier/GradientTint.kt (1)
20-26: 중복된 주석을 제거하세요.새로 추가된 KDoc(lines 9-19)이 이미 파라미터들을 충분히 설명하고 있으므로, 함수 내부의 인라인 주석(lines 21-24)은 중복입니다.
♻️ 제안하는 수정
fun Modifier.gradientTint( - /* - * brush: 그라데이션 색상을 적용할 브러시 - * blendMode: 그라데이션 색상을 적용할 때 사용할 블렌드 모드 - * */ brush: Brush, blendMode: BlendMode = BlendMode.SrcIn ): Modifier = this.graphicsLayer(alpha = 0.99f)design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt (5)
130-139: 주석 처리된 코드를 제거하세요.이전 구현이 주석 처리되어 있습니다. 버전 관리 시스템에서 히스토리를 추적할 수 있으므로, 사용하지 않는 코드는 삭제하는 것이 좋습니다.
348-354:resetAndDismiss()함수를 일관되게 사용하세요.배경 클릭 핸들러와 뒤로가기 버튼(lines 411-414)에서 동일한 로직(
text = "",isEditMode = false,onDismiss())이 중복됩니다.resetAndDismiss()함수가 이미 정의되어 있으므로 이를 활용하세요.단, 앞서 언급한
LaunchedEffect(visible)수정을 적용한 후에 사용해야 합니다.♻️ 제안하는 수정 (배경 클릭)
.noRippleClickable { - text = "" - isEditMode = false - onDismiss() + resetAndDismiss() }
300-301: 사용되지 않는uriHandler변수.
uriHandler가 선언되었지만 사용되지 않습니다. 불필요한 변수는 제거하세요.♻️ 제안하는 수정
- // uri 핸들러 - val uriHandler = LocalUriHandler.current
LocalUriHandlerimport도 함께 제거하세요.
247-248: Regex 객체 생성 최적화를 고려하세요.
Regex객체가 매 리컴포지션마다 생성됩니다.searchTerm이 변경될 때만 재생성하도록remember를 사용할 수 있습니다.♻️ 제안하는 수정
+ val regex = remember(searchTerm) { + Regex("(?i)${Regex.escape(searchTerm)}") + } - val regex = Regex("(?i)${Regex.escape(searchTerm)}")단,
HighlightedText가 내부 컴포저블이므로 영향은 제한적입니다.
306-309: 에러 처리 로직 검토 필요.
runCatching의onFailure블록이 비어 있어 오류가 무시됩니다.onLinkClick은 콜백이므로 예외 발생 가능성이 낮지만, 필요시 로깅을 추가하거나runCatching을 제거하는 것을 고려하세요.feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.kt (2)
68-101: 상태 업데이트를 단일 ViewModel 함수로 묶어 일관성 확보
여러update*호출이 분산돼 있어 중간 상태가 노출되거나 불필요한 재구성이 발생할 수 있습니다. 선택 로직과getSharedFolders()호출을 ViewModel의 단일 메서드로 캡슐화해 원자적으로 처리하는 쪽을 권장합니다.
129-163: 하드코딩 문자열을 string resource로 분리 권장
"나의 폴더","공유받은 폴더"가 코드에 직접 박혀 있어 i18n 및 UI 테스트에 불리합니다.stringResource(R.string...)로 분리하면 관리가 쉬워집니다.
design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt
Outdated
Show resolved
Hide resolved
| // 체크가 안된 항목만 체크 가능 | ||
| .noRippleClickable( | ||
| enabled = selectedOption != selectedText, | ||
| onClick = onClick | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "fun\s+noRippleClickable\b" -S --context 10Repository: LinkYou-2025/LinkU_Android
Length of output: 52
🏁 Script executed:
rg "noRippleClickable" -t kt --context 10Repository: LinkYou-2025/LinkU_Android
Length of output: 96
🏁 Script executed:
rg "noRippleClickable" --type-list | grep -i kotlinRepository: LinkYou-2025/LinkU_Android
Length of output: 459
🏁 Script executed:
rg "noRippleClickable" -A 15 --color neverRepository: LinkYou-2025/LinkU_Android
Length of output: 50382
🏁 Script executed:
cat design/src/main/java/com/example/design/modifier/NoRippleClickable.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 1353
🏁 Script executed:
sed -n '190,210p' feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 499
noRippleClickable 사용 시 명시적 접근성 Semantics 추가 권장
noRippleClickable 구현부는 role, onClickLabel 등 접근성 semantics를 올바르게 보존하고 있습니다. 다만 이 메뉴 항목(lines 197-201)에서는 role 파라미터가 명시되지 않아, TalkBack 사용자에게 요소의 역할이 명확하게 전달되지 않을 수 있습니다.
.noRippleClickable(enabled = selectedOption != selectedText, role = Role.Button, onClick = onClick)
와 같이 role 파라미터를 추가하여 더 나은 접근성 경험을 제공하시기 바랍니다.
🤖 Prompt for AI Agents
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.kt`
around lines 197 - 201, The clickable menu item using noRippleClickable (where
enabled = selectedOption != selectedText and onClick = onClick) lacks an
explicit accessibility role; update the call to pass the semantics role (e.g.,
Role.Button) — i.e., add the role parameter to noRippleClickable so TalkBack
users receive the proper element role (you can also include onClickLabel if
desired for a more descriptive announcement).
gradle/libs.versions.toml
Outdated
| androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation", version.ref = "foundation" } | ||
| androidx-compose-testing = { group = "androidx.xr.compose", name = "compose-testing", version.ref = "composeTesting" } | ||
| androidx-tools-core = { group = "androidx.privacysandbox.tools", name = "tools-core", version.ref = "toolsCore" } | ||
| androidx-compose-foundation-layout = { group = "androidx.compose.foundation", name = "foundation-layout", version.ref = "foundationLayout" } |
There was a problem hiding this comment.
git 충돌이 있어서, 아침에 이 부분만 수정해서 아침에 커밋 푸쉬했습니다.
| enabled: Boolean = true, | ||
| onClickLabel: String? = null, | ||
| role: Role? = null, | ||
| onClick: () -> Unit |
There was a problem hiding this comment.
여기 변경된 파라미터에 맞춰서 수정하겠습니다.
|
|
||
| // 입력 변화 디바운스 수집 (2자 이상 + 350ms) | ||
| // - mapLatest: 새 입력이 오면 이전 요청(코루틴 Job) 자동 취소 → 레이스 방지 | ||
| // 닫힐 때 상태 초기화 로직 (visible이 false가 될 때) |
There was a problem hiding this comment.
여기에 사용자가 탑 시트를 열었을 때, 자동으로 검색창에 커서가 가고 키보드 올라오도록 FocusRequester를 이용하도록 리펙토링 해보시는건 어떨까요?
| LaunchedEffect(text) { | ||
| snapshotFlow { text } | ||
| .map { it.trim() } | ||
| .filter { it.length >= 2 } |
There was a problem hiding this comment.
검색했을 때, 한 글자만 검색한 사용자 입장에서 왜 검색이 안되지? 싶을 수 있어서, 디자이너와 상의 후 관련 ui 추가하는 건 어떠실까요?
There was a problem hiding this comment.
검색했을 때, 한 글자만 검색한 사용자 입장에서 왜 검색이 안되지? 싶을 수 있어서, 디자이너와 상의 후 관련 ui 추가하는 건 어떠실까요?
윤다인과 이다현의 TODO. 한글자일 때 비활성화+상태메시지 ui
| onSelectSharedFolders: () -> Unit | ||
| ){ | ||
| // 선택된 폴더 | ||
| val selectedText = if (isSharedFolders) "공유받은 폴더" else "나의 폴더" |
There was a problem hiding this comment.
enum으로 분리하는게 깔끔할 것 같아용
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt (1)
400-414: 클릭 가능한 아이콘에 접근성 라벨을 추가해주세요.뒤로가기 및 검색어 지우기 버튼이
.noRippleClickable등으로 클릭 가능하지만contentDescription = null로 설정되어 있어 스크린리더에서 읽히지 않습니다. Jetpack Compose 접근성 가이드라인에 따르면, 클릭 가능한 Icon/Image 요소는 반드시 의미있는contentDescription을 제공해야 합니다.제안 변경
Icon( modifier = Modifier .height(40.dp) .noRippleClickable (onClick = onDismiss), tint = colors.gray[600], painter = painterResource(id = R.drawable.ic_back), - contentDescription = null + contentDescription = "뒤로가기" )if(text.isNotEmpty()){ Image( modifier = Modifier .padding(end = 18.dp) .size(18.dp) .noRippleClickable { text = "" }, painter = painterResource(R.drawable.ic_text_clear), - contentDescription = null + contentDescription = "검색어 지우기" ) }
🧹 Nitpick comments (2)
design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt (1)
129-138: 주석 처리된 이전 디바운스 블록은 제거하는 편이 좋습니다.
현재 구현이 있으므로 주석 블록은 혼동만 유발합니다.🧹 제안 변경
-/* - LaunchedEffect(Unit) { - snapshotFlow { text } - .map { it.trim() } - .filter { it.length >= 2 } // 2자 이상일 때만 - .debounce(350) // 350ms 주기 탐색 - .distinctUntilChanged() - .mapLatest(onQueryChange) - .collect { /* no-op */ } - }*/feature/home/src/main/java/com/example/home/screen/HomeScreen.kt (1)
203-205: itemsToRender/titleText 중복 선언과 하드코딩 이름은 정리 권장합니다.
바깥 titleText가 “세나님”으로 고정되고 내부에서 다시 선언되어 혼동 여지가 있습니다. 하나만 유지하고 userName 기반으로 통일하세요.♻️ 제안 변경
- val titleText = if (showRecs) "세나님의 오늘에 어울리는 콘텐츠예요!" + val titleText = if (showRecs) "${userName}님의 오늘에 어울리는 콘텐츠예요!" else "${userName}님이 최근에 열람한 링크" @@ - val itemsToRender = if (showRecs) recommendedLinks else recentLinks - val titleText = if (showRecs) "${userName}님의 오늘에 어울리는 콘텐츠예요!" - else "${userName}님이 최근에 열람한 링크"Also applies to: 257-264
ugmin1030
force-pushed
the
feature/#44
branch
from
c2f8ac9 to
9834a5c
Compare
- `feature/file/build.gradle.kts`에서 `androidx.compose.foundation.layout` 의존성을 `androidx.compose.foundation`으로 변경
- `LinksGrid.kt` 내 삭제 확인 모달의 버튼 텍스트를 "삭제" -> "삭제하기", "취소" -> "취소하기"로 변경 - 미사용 임포트(`lifecycleScope`, `rememberCoroutineScope`, `launch` 등) 제거 및 임포트 최적화
- `FileBottomSheet`에 0.5 투명도의 블랙 딤 컬러(`scrimColor`) 적용 - 텍스트 간격 조정을 위해 `Spacer` 추가 및 불필요한 `lineHeight`, `padding` 제거
- `animateFloatAsState`의 `label` 파라미터에 "화살표 회전 애니메이션" 명시 및 `targetValue` 네임드 파라미터 적용
- `LinkItemLayout`에 그림자 효과(`shadow`)를 적용하고 기존 `shadowElevation` 속성 제거 - 태그 UI 레이아웃 수정: `padding` 값 조정, 폰트 크기 변경(10sp -> 12sp), `includeFontPadding = false` 적용 - 컴포넌트 간 간격 조정을 위해 `Spacer` 추가 및 기존 `padding` 로직 수정 - 불필요한 `showDialog` 상태 및 미사용 import 제거 - 클릭 이벤트 로직 내 불필요한 null safety 체크 최적화
- `selectedLinks`를 `mutableStateListOf`로 변경하여 상태 추적 지원 - 바텀시트 '추가' 버튼 활성화 상태(`isReady`) 조건 추가 (선택된 링크가 있을 때만 활성화) - 링크 아이템 레이아웃 수정: `offset`, `padding`, `shape` 및 정렬 조정 - 체크박스 미선택 시 색상(`uncheckedColor`) 변경 - 링크 선택 시 데이터 정합성을 위해 `onOkay` 콜백 내 `selectedLinks.clear()` 로직 추가
- `ShareFolderIcon`, `LockFolderIcon`, `PencilIcon`, `BookMarkStar` 함수에 `internal` 접근 제한자 적용
`LaunchedEffect`의 key를 `text`에서 `Unit`으로 변경하여, 컴포저블이 처음 생성될 때만 검색 로직 관련 Flow가 생성되도록 수정했습니다. 이를 통해 불필요한 초기 호출을 방지합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/file/src/main/java/com/example/file/ui/bottom/sheet/LinkCategorizationBottomSheet.kt (1)
101-138:⚠️ Potential issue | 🟠 Major
checked상태와selectedLinks의 동기화 문제로 UI 불일치가 발생합니다.아이템마다
remember로 초기화된checked는selectedLinks.clear()이후에도 true로 유지되며, 목록이 재구성될 때 remember 상태가 위치 기반으로 재사용되어 상태가 꼬입니다.checked를selectedLinks.contains(link)에서 파생시키고, 중복된 토글 로직을 단일 함수로 통합하세요. 또한items()에 안정적인 key를 지정하여 remember 상태 뒤틀림을 방지하는 것을 권장합니다.✅ 제안 수정안
- items(links) { - val link = it + items(links, key = { it.id }) { + val link = it val title = it.title val url = it.url val icon = domainLogoPainterOrNull(it.url)?:painterResource(R.drawable.link_categorization_default) val img = painterResource(R.drawable.link_categorization_default)//it.img - var checked by remember { mutableStateOf(false)} + val checked = selectedLinks.contains(link) + val toggleSelection = { + if (selectedLinks.contains(link)) { + selectedLinks.remove(link) + } else { + selectedLinks.add(link) + } + } Row( modifier = Modifier .height(60.dp) .offset(x = (-20).dp) .noRippleClickable{ - if(checked){ - Log.d("LinkCategorizationBottomSheet", "checked: $it") - selectedLinks.remove(link) - checked = selectedLinks.contains(link) - } else { - Log.d("LinkCategorizationBottomSheet", "checked: $it") - selectedLinks.add(link) - checked = selectedLinks.contains(link) - } + Log.d("LinkCategorizationBottomSheet", "checked: $it") + toggleSelection() }, horizontalArrangement = Arrangement.spacedBy(10.dp), verticalAlignment = Alignment.CenterVertically ) { Checkbox( checked = checked, onCheckedChange = { - if(checked){ - Log.d("LinkCategorizationBottomSheet", "checked: $it") - selectedLinks.remove(link) - checked = selectedLinks.contains(link) - } else { - Log.d("LinkCategorizationBottomSheet", "checked: $it") - selectedLinks.add(link) - checked = selectedLinks.contains(link) - } + Log.d("LinkCategorizationBottomSheet", "checked: $it") + toggleSelection() },(주:
it.id는LinkItemInfo의 실제 고유 식별자로 대체하세요)feature/file/src/main/java/com/example/file/ui/item/LinkItemLayout.kt (1)
150-166:⚠️ Potential issue | 🟡 MinorSurface 모서리와 그림자 모양 일치 필요
shape파라미터가 주석 처리되어 있으면 Surface의 배경과 클리핑이 기본값(직사각형)으로 적용됩니다. 현재.shadow()수정자에서 18dp 라운드를 지정했지만, Surface 자체는 직사각형으로 렌더링되어 모서리와 그림자 outline이 불일치합니다.shape = RoundedCornerShape(18.dp)를 활성화하여 Surface의 배경 클리핑도 같은 라운드로 일치시키세요.🎯 모서리 일치용 수정안
Surface( modifier = Modifier .width(181.dp) .then(modifier) .shadow( elevation = 4.dp, shape = RoundedCornerShape(18.dp) ), - //shape = RoundedCornerShape(18.dp), + shape = RoundedCornerShape(18.dp), color = White, ) {
🤖 Fix all issues with AI agents
In
`@feature/file/src/main/java/com/example/file/ui/bottom/sheet/LinkCategorizationBottomSheet.kt`:
- Around line 80-90: 사용자가 폴더를 선택하지 않았을 때
requireNotNull(folderStateViewModel.selectedBottomFolder?.folderId)로 인해 앱이 크래시
발생하므로, selectedBottomFolder의 유효성을 검사하도록 수정하세요: UI 상태 검사 지점인 isReady에
folderStateViewModel.selectedBottomFolder != null 조건을 추가하거나, 더 안전하게 onOkay
블록(함수명/속성: onOkay, scope.launch) 시작부에서 val folderId =
folderStateViewModel.selectedBottomFolder?.folderId ?: return@launch 식의
early-return을 추가해 folderId가 없으면 작업을 중단하고 selectedLinks 순회나 updateLinkFolder 호출을
하지 않도록 하세요; 관련 식별자: selectedBottomFolder, isReady, onOkay, folderStateViewModel,
selectedLinks, updateLinkFolder, requireNotNull.
🧹 Nitpick comments (2)
design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt (1)
342-355: resetAndDismiss 재사용으로 중복 제거 가능.
동일 초기화 로직을 한 곳으로 모으면 유지보수성이 좋아집니다.feature/file/src/main/java/com/example/file/ui/item/LinkItemLayout.kt (1)
135-147: 터치 피드백 제거로 인해 클릭 인지가 약해질 수 있습니다.
indication = null로 기본 리플이 사라져 인터랙션 피드백이 없어집니다. 의도된 디자인이 아니라면 기본 indication을 살리거나 별도 피드백을 넣는 편이 UX에 유리합니다.✅ 기본 리플 복원 예시
- indication = null, interactionSource = remember { MutableInteractionSource() },
| isReady = selectedLinks.isNotEmpty(), | ||
| visible = folderStateViewModel.linkCategorizationBottomSheetVisible, | ||
| onOkay = { | ||
| scope.launch{ | ||
| selectedLinks.map { | ||
| fileViewModel.updateLinkFolder( | ||
| it, | ||
| folderStateViewModel.selectedBottomFolder?.folderId!! | ||
| ) | ||
| val folderId = requireNotNull(folderStateViewModel.selectedBottomFolder?.folderId) | ||
|
|
||
| selectedLinks.forEach { | ||
| fileViewModel.updateLinkFolder(it, folderId) | ||
| } | ||
|
|
||
| selectedLinks.clear() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "LinkCategorizationBottomSheet.kt" -type fRepository: LinkYou-2025/LinkU_Android
Length of output: 164
🏁 Script executed:
wc -l ./feature/file/src/main/java/com/example/file/ui/bottom/sheet/LinkCategorizationBottomSheet.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 168
🏁 Script executed:
cat -n ./feature/file/src/main/java/com/example/file/ui/bottom/sheet/LinkCategorizationBottomSheet.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 11453
🏁 Script executed:
find . -name "FolderStateViewModel.kt" -type fRepository: LinkYou-2025/LinkU_Android
Length of output: 162
🏁 Script executed:
wc -l ./feature/file/src/main/java/com/example/file/viewmodel/folder/state/FolderStateViewModel.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 166
🏁 Script executed:
cat -n ./feature/file/src/main/java/com/example/file/viewmodel/folder/state/FolderStateViewModel.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 6378
selectedBottomFolder가 null이면 앱이 크래시됩니다.
selectedBottomFolder는 nullable 타입(FolderSimpleInfo?)으로 선언되어 있고, isReady는 선택된 링크 여부만 확인하므로 폴더 유효성을 검증하지 않습니다. 사용자가 폴더를 선택하지 않은 상태에서 버튼을 클릭하면 requireNotNull(folderStateViewModel.selectedBottomFolder?.folderId)에서 NPE가 발생하여 앱이 강제 종료됩니다. 폴더 존재 여부를 isReady에 추가하거나 onOkay에서 안전한 early-return 처리를 하세요.
🛡️ 제안 수정안
- isReady = selectedLinks.isNotEmpty(),
+ isReady = selectedLinks.isNotEmpty() &&
+ folderStateViewModel.selectedBottomFolder?.folderId != null,
visible = folderStateViewModel.linkCategorizationBottomSheetVisible,
onOkay = {
scope.launch{
- val folderId = requireNotNull(folderStateViewModel.selectedBottomFolder?.folderId)
+ val folderId =
+ folderStateViewModel.selectedBottomFolder?.folderId ?: return@launch
selectedLinks.forEach {
fileViewModel.updateLinkFolder(it, folderId)
}
selectedLinks.clear()
}
},🤖 Prompt for AI Agents
In
`@feature/file/src/main/java/com/example/file/ui/bottom/sheet/LinkCategorizationBottomSheet.kt`
around lines 80 - 90, 사용자가 폴더를 선택하지 않았을 때
requireNotNull(folderStateViewModel.selectedBottomFolder?.folderId)로 인해 앱이 크래시
발생하므로, selectedBottomFolder의 유효성을 검사하도록 수정하세요: UI 상태 검사 지점인 isReady에
folderStateViewModel.selectedBottomFolder != null 조건을 추가하거나, 더 안전하게 onOkay
블록(함수명/속성: onOkay, scope.launch) 시작부에서 val folderId =
folderStateViewModel.selectedBottomFolder?.folderId ?: return@launch 식의
early-return을 추가해 folderId가 없으면 작업을 중단하고 selectedLinks 순회나 updateLinkFolder 호출을
하지 않도록 하세요; 관련 식별자: selectedBottomFolder, isReady, onOkay, folderStateViewModel,
selectedLinks, updateLinkFolder, requireNotNull.
# Conflicts: # gradle/libs.versions.toml
- 비밀번호 재설정, 구분선, 회원가입 버튼의 배치를 `Box`의 오프셋 방식에서 `Row`를 사용한 정렬 방식으로 변경 - `Arrangement.spacedBy`를 사용하여 요소 간 간격 조정 및 중앙 정렬 적용
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/login/src/main/java/com/example/login/ui/screen/EmailLoginScreen.kt (1)
236-238:⚠️ Potential issue | 🟡 Minor사용되지 않는 변수 (Dead code)
resetStartPos,dividerStartPos,signUpStartPos변수들이 정의되어 있지만, Row 기반 레이아웃으로 리팩토링되면서 더 이상 사용되지 않습니다. 이 변수들을 삭제해 주세요.🧹 제안된 수정 사항
- // 🔑 비율 기반 가로 위치 계산 - // 디자인 기준 너비 412 대비 현재 화면의 비율 지점 - val resetStartPos = (101.scaler) // 비밀번호 재설정 시작점 - val dividerStartPos = (220.scaler) // | 시작점 - val signUpStartPos = (247.scaler) // 회원가입 시작점 - Row(
🧹 Nitpick comments (1)
feature/login/src/main/java/com/example/login/ui/screen/EmailLoginScreen.kt (1)
245-245: 단위 일관성 확인 필요
Arrangement.spacedBy(25.dp, ...)에서25.dp를 사용하고 있는데, 이 파일의 다른 부분에서는 반응형 스케일링을 위해.scaler를 사용하고 있습니다. 의도적인 고정값이 아니라면 일관성을 위해25.scaler로 변경하는 것을 고려해 주세요.♻️ 제안된 수정 사항
- horizontalArrangement = Arrangement.spacedBy(25.dp, alignment = Alignment.CenterHorizontally), + horizontalArrangement = Arrangement.spacedBy(25.scaler, alignment = Alignment.CenterHorizontally),
- `BottomFolderListMenu`를 UI 로직과 레이아웃(`BottomFolderListMenuLayout`)으로 분리 - `DropdownMenuItem` 대신 커스텀 로직이 포함된 `BottomFolderListMenuRow` 컴포넌트 사용 - 메뉴 내부에 `verticalScroll`을 적용하여 최대 높이(264dp) 초과 시 스크롤 가능하도록 수정 - 스크롤 바 구현을 위한 상태 관리 로직 추가 (추후 수정 대비 주석 포함) - 프리뷰 확인을 위한 테스트 데이터(`folderSimpleInfoList`) 및 `BottomFolderListMenuLayoutTest` 추가
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt (2)
182-224: 로컬@Composable함수 선언 — 비관용적 패턴 및 Debug 로그 잔존두 가지 문제가 있습니다:
로컬 컴포저블:
ScrollBar를BottomFolderListMenuLayout내부에 로컬 함수로 선언하면, Compose 컴파일러가 리컴포지션 스코프와 positional memoization을 올바르게 처리하지 못할 수 있습니다. 파일 최상위private fun으로 추출하는 것이 관용적입니다.프로덕션
Log.d코드: lines 184–186의 디버그 로그는 scrollbar 활성화 시 스크롤마다 호출됩니다.🔧 Proposed refactor
- `@Composable` - fun ScrollBar(modifier: Modifier) { - Log.d("SB", "value=${menuScrollState.value}, max=${menuScrollState.maxValue}, track=$trackHeight, thumb=$thumbHeight, off=$thumbTopPadding") - Log.d("SB", "thumbTopPadding=$thumbTopPadding, thumbHeight=$thumbHeight") - Log.d("SB", "scrollable=$scrollable") - // ... - }
ScrollBar를 별도private최상위 컴포저블로 추출하고, 필요한 값을 파라미터로 전달하세요. 추후 수정 시Log.d호출도 제거하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt` around lines 182 - 224, Extract the local `@Composable` ScrollBar out of BottomFolderListMenuLayout into a private top-level composable (private `@Composable` fun ScrollBar(...)) and remove the Log.d debug calls; add explicit parameters for all state it reads (e.g., menuScrollState.value/maxValue, trackHeight, thumbHeight, thumbTopPadding, scrollable and Modifier) so callers pass those values in, and update the call site in BottomFolderListMenuLayout to pass the required arguments; ensure no Log.d calls remain in the new top-level ScrollBar.
120-121: 메뉴 크기 상수가 분산 선언되어 불일치 위험
menuHeight = 264f(line 120)는 로컬 변수로 선언되어 있지만, 실제heightIn(max = 264.dp)는 lines 243, 262에 다시 하드코딩되어 있습니다. 한 곳만 수정하면 값이 달라지는 문제가 발생할 수 있습니다. 주석 처리된 lines 99–101처럼MENU_ITEM_HEIGHT(line 293)와 동일하게 파일 수준private const로 통일하세요.🔧 Proposed refactor
-// private const val BOTTOM_FOLDER_LIST_MENU_WIDTH = 205f -// private const val BOTTOM_FOLDER_LIST_MENU_HEIGHT = 264F +private const val BOTTOM_FOLDER_LIST_MENU_WIDTH = 205f +private const val BOTTOM_FOLDER_LIST_MENU_HEIGHT = 264f private const val MENU_ITEM_HEIGHT = 25- val menuHeight = 264f - val menuWidth = 205f + val menuHeight = BOTTOM_FOLDER_LIST_MENU_HEIGHT + val menuWidth = BOTTOM_FOLDER_LIST_MENU_WIDTH- .heightIn(max = 264.dp) // line 243 + .heightIn(max = BOTTOM_FOLDER_LIST_MENU_HEIGHT.dp)- .heightIn(max = 264.dp) // line 262 + .heightIn(max = BOTTOM_FOLDER_LIST_MENU_HEIGHT.dp)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt` around lines 120 - 121, The menu size constants are scattered (local val menuHeight/menuWidth and hardcoded heightIn(max = 264.dp)) causing drift; consolidate them into file-level constants (e.g., private const val MENU_HEIGHT = 264f and MENU_WIDTH = 205f or corresponding dp constants) and replace all local uses of menuHeight, menuWidth and hardcoded heightIn(max = 264.dp) with these constants (and update MENU_ITEM_HEIGHT if needed) so every reference in BottomFolderListMenu (including the places using heightIn and the existing MENU_ITEM_HEIGHT) uses the single file-level constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt`:
- Around line 148-180: thumbHeight and thumbTopPadding capture plain Ints
outside their derivedStateOf lambdas which fixes their values at creation;
change each derivedStateOf to read the latest composable State inside the lambda
(e.g., read menuScrollState.value and menuScrollState.maxValue or the equivalent
live State instead of using external Int snapshots like
maxScrollValue/currentScrollValue) so the derived computations (in thumbHeight
and thumbTopPadding) re-run on scroll updates; update references inside the
lambdas to use current menuHeight, trackHeight and menuScrollState.* directly
and remove the external captured Int usage.
- Around line 119-145: The code reads menuScrollState.value and
menuScrollState.maxValue in composition (currentScrollValue and maxScrollValue)
and defines unused scrollBarWidthWeight and scrollBarHeightWeight, causing
unnecessary recompositions; remove the direct reads and the two dead variables
(scrollBarWidthWeight, scrollBarHeightWeight) from BottomFolderListMenu.kt, and
if you need scroll values later wrap reads inside a derivedStateOf or read them
only inside the scrollbar implementation (e.g., use derivedStateOf {
menuScrollState.value } or derivedStateOf { menuScrollState.maxValue } within
the scrollbar composable) so the rest of BottomFolderListMenuLayout no longer
observes scroll snapshot state during composition.
---
Nitpick comments:
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt`:
- Around line 182-224: Extract the local `@Composable` ScrollBar out of
BottomFolderListMenuLayout into a private top-level composable (private
`@Composable` fun ScrollBar(...)) and remove the Log.d debug calls; add explicit
parameters for all state it reads (e.g., menuScrollState.value/maxValue,
trackHeight, thumbHeight, thumbTopPadding, scrollable and Modifier) so callers
pass those values in, and update the call site in BottomFolderListMenuLayout to
pass the required arguments; ensure no Log.d calls remain in the new top-level
ScrollBar.
- Around line 120-121: The menu size constants are scattered (local val
menuHeight/menuWidth and hardcoded heightIn(max = 264.dp)) causing drift;
consolidate them into file-level constants (e.g., private const val MENU_HEIGHT
= 264f and MENU_WIDTH = 205f or corresponding dp constants) and replace all
local uses of menuHeight, menuWidth and hardcoded heightIn(max = 264.dp) with
these constants (and update MENU_ITEM_HEIGHT if needed) so every reference in
BottomFolderListMenu (including the places using heightIn and the existing
MENU_ITEM_HEIGHT) uses the single file-level constants.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt
| // ----------------------------스크롤 바 추후 수정---------------------------- | ||
| val menuHeight = 264f | ||
| val menuWidth = 205f | ||
|
|
||
| // 스크롤 바 너비비 = 스크롤 바 너비 / 메뉴 너비 | ||
| val scrollBarWidthWeight = 4f / menuWidth | ||
|
|
||
| // 스크롤 바 길이비 = 메뉴 길이 / 전체 아이템 총 길이 | ||
| val scrollBarHeightWeight = menuHeight / (subFolders.size * MENU_ITEM_HEIGHT) | ||
|
|
||
| // 트랙(스크롤바가 움직이는 레일) 높이 | ||
| var trackHeight by remember { mutableIntStateOf(0) } | ||
|
|
||
|
|
||
| val menuScrollState = rememberScrollState() | ||
|
|
||
| LaunchedEffect(bottomMenuExpanded) { | ||
| if(bottomMenuExpanded) | ||
| menuScrollState.scrollTo(0) | ||
| } | ||
|
|
||
| val scrollable by remember { | ||
| derivedStateOf { menuScrollState.maxValue > 0 } | ||
| } | ||
|
|
||
| val currentScrollValue = menuScrollState.value | ||
| val maxScrollValue = menuScrollState.maxValue |
There was a problem hiding this comment.
스크롤 상태의 불필요한 컴포지션 단계 읽기로 인한 과도한 리컴포지션
Lines 144–145에서 menuScrollState.value와 menuScrollState.maxValue를 컴포지션 단계에서 일반 Int로 읽고 있습니다. ScrollState.value는 Compose의 State로 관리되므로, 컴포지션 단계에서 읽으면 스냅샷 관찰이 등록되어 스크롤 위치가 변경될 때마다 리컴포지션이 트리거됩니다. 두 값 모두 현재 주석 처리된 scrollbar에서만 사용되므로, 실제 UI 변경 없이 스크롤할 때마다 BottomFolderListMenuLayout 전체가 리컴포지션됩니다.
또한 scrollBarWidthWeight (line 124), scrollBarHeightWeight (line 127)는 실제로 사용되지 않는 dead code입니다.
scrollbar가 아직 미완성이므로, 우선 두 읽기와 미사용 변수들을 제거하고 추후 수정 시 derivedStateOf 내부에서 상태를 읽도록 구현하세요:
🔧 Proposed fix
- // 스크롤 바 너비비 = 스크롤 바 너비 / 메뉴 너비
- val scrollBarWidthWeight = 4f / menuWidth
-
- // 스크롤 바 길이비 = 메뉴 길이 / 전체 아이템 총 길이
- val scrollBarHeightWeight = menuHeight / (subFolders.size * MENU_ITEM_HEIGHT)
-
- // 트랙(스크롤바가 움직이는 레일) 높이
var trackHeight by remember { mutableIntStateOf(0) }
val menuScrollState = rememberScrollState()
LaunchedEffect(bottomMenuExpanded) {
if(bottomMenuExpanded)
menuScrollState.scrollTo(0)
}
- val scrollable by remember {
- derivedStateOf { menuScrollState.maxValue > 0 }
- }
-
- val currentScrollValue = menuScrollState.value
- val maxScrollValue = menuScrollState.maxValue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt`
around lines 119 - 145, The code reads menuScrollState.value and
menuScrollState.maxValue in composition (currentScrollValue and maxScrollValue)
and defines unused scrollBarWidthWeight and scrollBarHeightWeight, causing
unnecessary recompositions; remove the direct reads and the two dead variables
(scrollBarWidthWeight, scrollBarHeightWeight) from BottomFolderListMenu.kt, and
if you need scroll values later wrap reads inside a derivedStateOf or read them
only inside the scrollbar implementation (e.g., use derivedStateOf {
menuScrollState.value } or derivedStateOf { menuScrollState.maxValue } within
the scrollbar composable) so the rest of BottomFolderListMenuLayout no longer
observes scroll snapshot state during composition.
| val thumbHeight by remember(trackHeight, maxScrollValue) { | ||
| derivedStateOf { | ||
| if (trackHeight <= 0) 0f | ||
| else if (maxScrollValue <= 0) 0f // 스크롤 불가면 전체(사실상 숨겨도 됨) | ||
| else { | ||
| val contentHeightPx = trackHeight + maxScrollValue | ||
| val ratio = trackHeight.toFloat() / contentHeightPx.toFloat() | ||
| // UX용 최소 높이(너무 작아지면 안 보임 방지) - 필요 없으면 삭제 가능 | ||
| /*val minThumbPx = (18.dp.value * (trackHeight / 264f)).roundToInt() | ||
| max((trackHeight * ratio).roundToInt(), minThumbPx)*/ | ||
| //(trackHeight * ratio).roundToInt() | ||
| //ratio | ||
| menuHeight * ratio | ||
| } | ||
| } | ||
| } | ||
|
|
||
| val thumbTopPadding by remember(trackHeight, maxScrollValue, currentScrollValue, thumbHeight) { | ||
| derivedStateOf { | ||
| if (trackHeight <= 0) 0f | ||
| else if (maxScrollValue <= 0) 0f | ||
| else { | ||
| val movableRange = (trackHeight - thumbHeight).coerceAtLeast(0F) | ||
| val ratio = currentScrollValue.toFloat() / maxScrollValue.toFloat() | ||
| val contentHeightPx = trackHeight + maxScrollValue | ||
| val thumbRatio = trackHeight.toFloat() / contentHeightPx.toFloat() | ||
| val r = (1 - thumbRatio) * ratio | ||
| //(movableRange * ratio).roundToInt() | ||
| //(trackHeight * ratio).roundToInt() | ||
| r | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
derivedStateOf 람다 외부에서 State를 캡처하는 버그 — 스크롤 시 thumb 위치가 업데이트되지 않음
thumbHeight(line 148)와 thumbTopPadding(line 165) 모두 derivedStateOf 람다 외부에서 maxScrollValue와 currentScrollValue를 일반 Int로 캡처합니다. derivedStateOf는 컴포저블 State 객체가 아닌 변수를 캡처할 경우 최초 생성 시 값을 고정하며, 이후 값이 바뀌어도 재계산되지 않습니다.
scrollbar 활성화 시 menuScrollState.value/maxValue를 람다 내부에서 직접 읽어야 합니다:
🔧 Proposed fix (추후 수정 시 적용)
- val currentScrollValue = menuScrollState.value
- val maxScrollValue = menuScrollState.maxValue
-
val thumbHeight by remember(trackHeight) {
derivedStateOf {
+ val maxScrollValue = menuScrollState.maxValue
if (trackHeight <= 0) 0f
else if (maxScrollValue <= 0) 0f
else {
val contentHeightPx = trackHeight + maxScrollValue
val ratio = trackHeight.toFloat() / contentHeightPx.toFloat()
menuHeight * ratio
}
}
}
- val thumbTopPadding by remember(trackHeight, maxScrollValue, currentScrollValue, thumbHeight) {
+ val thumbTopPadding by remember(trackHeight) {
derivedStateOf {
+ val maxScrollValue = menuScrollState.maxValue
+ val currentScrollValue = menuScrollState.value
if (trackHeight <= 0) 0f
else if (maxScrollValue <= 0) 0f
else {
val movableRange = (trackHeight - thumbHeight).coerceAtLeast(0F)
val ratio = currentScrollValue.toFloat() / maxScrollValue.toFloat()
val contentHeightPx = trackHeight + maxScrollValue
val thumbRatio = trackHeight.toFloat() / contentHeightPx.toFloat()
val r = (1 - thumbRatio) * ratio
r
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/BottomFolderListMenu.kt`
around lines 148 - 180, thumbHeight and thumbTopPadding capture plain Ints
outside their derivedStateOf lambdas which fixes their values at creation;
change each derivedStateOf to read the latest composable State inside the lambda
(e.g., read menuScrollState.value and menuScrollState.maxValue or the equivalent
live State instead of using external Int snapshots like
maxScrollValue/currentScrollValue) so the derived computations (in thumbHeight
and thumbTopPadding) re-run on scroll updates; update references inside the
lambdas to use current menuHeight, trackHeight and menuScrollState.* directly
and remove the external captured Int usage.
- `Modifier.innerRingShadow` 확장 함수 구현 - `Brush.radialGradient`를 사용하여 컴포넌트 가장자리에 부드러운 안쪽 테두리 그림자 효과 적용 - 그림자 색상(`shadowColor`) 및 두께(`edgeThickness`) 커스텀 매개변수 제공
- 기존 `changeSharing` 함수를 `folderToShare`(공개 전환)와 `folderToPrivate`(비공개 전환)로 분리 - `getNotCategorizationLinks` 함수 내 `_subFolders` 상태 확인을 위한 디버그 로그 추가 - 각 전환 함수 내 조건문 로직 수정 및 상세 로그 추가
- `FolderItemLayout` 내 텍스트 배경 아이콘에 `innerRingShadow` 효과 적용 - 불필요한 주석 코드 및 미사용 import 제거 - `TopFolderItemLayout` 및 `CategoryFolderItemLayout`에 즐겨찾기 아이콘 노출 여부 제어를 위한 `visibleBookmarked` 파라미터 추가 - `CategoryFolderItemLayout`의 공유 상태 아이콘 로직 개선 및 색상 변경 (color1 -> color2)
3 participants
📝 설명
✔️ PR 유형
어떤 변경 사항이 있나요?
📎 관련 이슈 번호
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
스타일